Test fix and sql support#24
Merged
Merged
Conversation
- Add better-sqlite3 and pg for database storage backends - Refactor store.ts into modular structure under store/ directory - Add PostgreSQL service to CI workflow for database tests - Fix agent-id test to validate agt- format with Crockford Base32 - Fix integration test prompts and approval handling - Remove orphaned OPENAI_RESPONSES_* configurations
- Dispose subagent sandbox after task_run completion - Add file system sync delay for resume test on CI - Improve test cleanup with retries and explicit sandbox disposal
Contributor
Author
|
Related: #1 |
CrazyBoyM
reviewed
Jan 21, 2026
Contributor
There was a problem hiding this comment.
🔍 PR #24 全面审查报告
基于对整个 Kode Agent SDK 架构、大规模部署场景、Serverless 兼容性的深度评估,对本 PR 进行全面审查。
一、PR 总体评价
评分: ⭐⭐⭐⭐ (4/5)
| 维度 | 评价 |
|---|---|
| 功能完整性 | ✅ SQLite/PostgreSQL 双后端,QueryableStore 接口完善 |
| 架构设计 | ✅ 混合存储策略合理,DB 存查询数据,FS 存高频写入 |
| 文档质量 | ✅ 1000+ 行详细文档,示例丰富 |
| 向后兼容 | ✅ Store 接口保持不变,现有代码无需修改 |
| 测试覆盖 | ✅ 单元测试完整,CI 集成 PostgreSQL |
| 代码质量 |
结论: 功能设计优秀,建议修复高优先级问题后合并。
二、必须修复的问题 (Blocking)
2.1 PostgresStore 初始化状态检查缺失
位置: src/infra/db/postgres/postgres-store.ts
问题: 如果数据库连接失败,initPromise 会 reject,但后续调用 saveMessages() 等方法时没有检查初始化状态,可能导致静默失败或难以追踪的错误。
当前代码:
constructor(connectionConfig: any, fileStoreBaseDir: string) {
this.pool = new Pool(connectionConfig);
this.fileStore = new JSONStore(fileStoreBaseDir);
this.initPromise = this.initialize().catch(err => {
this.initError = err;
throw err; // 这里抛出但没有被处理
});
}建议修复:
private async ensureInitialized(): Promise<void> {
await this.initPromise;
if (this.initError) {
throw new Error(`PostgresStore initialization failed: ${this.initError.message}`);
}
}
async saveMessages(agentId: string, messages: Message[]): Promise<void> {
await this.ensureInitialized(); // 每个公开方法都要加
// ... 原有逻辑
}2.2 connectionConfig 使用 any 类型
位置: src/infra/db/postgres/postgres-store.ts 构造函数
问题: 缺乏类型安全,错误的配置不会被编译期捕获。
建议修复 - 在 types.ts 中添加:
export interface PostgresConfig {
host: string;
port: number;
database: string;
user: string;
password: string;
ssl?: boolean | { rejectUnauthorized?: boolean };
max?: number; // 连接池最大连接数
idleTimeoutMillis?: number; // 空闲连接超时
connectionTimeoutMillis?: number; // 连接超时
}三、强烈建议修复 (High Priority)
3.1 连接池默认配置与错误监听
建议改进:
constructor(config: PostgresConfig, fileStoreBaseDir: string) {
const poolConfig = {
...config,
max: config.max ?? 10,
idleTimeoutMillis: config.idleTimeoutMillis ?? 30000,
connectionTimeoutMillis: config.connectionTimeoutMillis ?? 5000,
};
this.pool = new Pool(poolConfig);
// 监听连接池错误,避免未处理的异常
this.pool.on('error', (err) => {
console.error('[PostgresStore] Unexpected pool error:', err);
});
// ...
}3.2 混合存储一致性风险
问题: 数据库写入成功但文件系统写入失败会导致状态不一致。
建议: 添加操作日志用于恢复判断(可在后续 PR 实现):
// 记录关键操作状态
interface OperationLog {
id: string;
agentId: string;
operation: 'save_messages' | 'save_info' | 'fork';
status: 'started' | 'db_committed' | 'fs_committed' | 'completed';
startedAt: Date;
}3.3 文档中 agentId 格式不一致
位置: docs/database.md
问题: 示例代码中仍使用旧的 agt: 格式,应统一为 agt-。
- const messages = await store.queryMessages({ agentId: 'agt:abc123' });
+ const messages = await store.queryMessages({ agentId: 'agt-abc123' });四、建议后续改进 (Follow-up)
4.1 健康检查接口
interface StoreHealthStatus {
healthy: boolean;
database: { connected: boolean; latencyMs?: number };
fileSystem: { writable: boolean };
}
async healthCheck(): Promise<StoreHealthStatus>;4.2 一致性检查接口
async checkConsistency(agentId: string): Promise<{
consistent: boolean;
issues: string[];
}>;4.3 Store 工厂函数
function createStore(config: StoreConfig): Store {
switch (config.type) {
case 'json': return new JSONStore(config.baseDir);
case 'sqlite': return new SqliteStore(config.dbPath);
case 'postgres': return new PostgresStore(config);
}
}五、与 Issue #1 的关系
本 PR 正是 Issue #1 (云端数据库集成) 的解决方案。建议:
六、大规模部署场景下的补充建议
基于对 Kode SDK 在大规模 To C 场景下的评估,建议在后续版本中考虑:
6.1 分布式锁支持
当多个 Worker 实例可能同时操作同一 Agent 时:
// PostgreSQL Advisory Lock
async acquireAgentLock(agentId: string): Promise<() => Promise<void>>;6.2 批量操作优化
大量 Fork 场景下的性能优化:
async batchFork(agentId: string, count: number): Promise<string[]>;6.3 指标采集
interface StoreMetrics {
operations: { saves: number; loads: number; queries: number };
performance: { avgLatencyMs: number };
storage: { totalAgents: number; dbSizeBytes: number };
}七、审查结论
| 检查项 | 状态 |
|---|---|
| 初始化状态检查 | ❌ 需修复 |
| 类型定义 | ❌ 需修复 |
| 连接池配置 | |
| 文档一致性 | |
| 功能完整性 | ✅ 通过 |
| 测试覆盖 | ✅ 通过 |
| 向后兼容 | ✅ 通过 |
建议: 修复 2 个 blocking 问题后合并,其他问题可作为 follow-up。
八、Best Next Works 指南
阶段 1: 本 PR 修复 (合并前)
- 添加
ensureInitialized()检查到所有公开方法 - 添加
PostgresConfig类型定义 - 添加连接池错误监听
- 修复文档中的 agentId 格式
阶段 2: 紧随 PR (1-2 周内)
- 添加
healthCheck()方法 - 添加
checkConsistency()方法 - 创建
createStore()工厂函数 - 在 Issue #1 添加回复并关闭
阶段 3: 后续迭代 (1-2 个月)
- 实现分布式锁 (PostgreSQL Advisory Lock)
- 添加 Store 指标采集
- 评估 Redis Store 需求
- 更新 README 添加适用场景说明
感谢 @Gui-Yue 的贡献!这是一个重要的功能增强,推进了 Kode SDK 的多实例部署和企业级应用。
Gui-Yue
force-pushed
the
test_fix_and_sql_support
branch
from
January 22, 2026 07:35
7f20cf1 to
ab0fdfa
Compare
Contributor
Author
✅ 二、必须修复的问题 (Blocking) - 已全部修复2.1 PostgresStore 初始化状态检查
2.2 connectionConfig 类型安全
✅ 三、强烈建议修复 (High Priority) - 已全部修复3.1 连接池默认配置与错误监听const poolConfig = {
...config,
port: config.port ?? 5432,
max: config.max ?? 10,
idleTimeoutMillis: config.idleTimeoutMillis ?? 30000,
connectionTimeoutMillis: config.connectionTimeoutMillis ?? 5000,
};
this.pool.on('error', (err) => {
console.error('[PostgresStore] Unexpected pool error:', err.message);
});3.2 混合存储一致性风险
3.3 文档 agentId 格式
✅ 四、建议后续改进 (Follow-up) - 已提前实现
✅ 六、大规模部署建议 - 已提前实现
📊 变更统计
📁 新增文件
🔄 七、审查结论更新
|
…and distributed lock ## Blocking fixes - Add ensureInitialized() check to all PostgresStore public methods - Replace connectionConfig: any with typed PostgresConfig interface - Add connection pool default config and error listener ## New features (ExtendedStore interface) - healthCheck(): database and filesystem health status - checkConsistency(): verify data consistency between DB and filesystem - getMetrics(): operation counts, latency stats, storage stats - acquireAgentLock(): distributed lock (PostgreSQL advisory lock / memory lock) - batchFork(): optimized bulk agent forking ## Other changes - Add createStore() and createExtendedStore() factory functions - Fix agentId format in docs and tests (agt: → agt-) - Add comprehensive tests for new features
Gui-Yue
force-pushed
the
test_fix_and_sql_support
branch
from
January 22, 2026 08:05
ab0fdfa to
19e3a22
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: 添加 SQLite/PostgreSQL 持久化支持 & 资源清理与测试稳定性修复
概述
本 PR 包含三部分改动:
变更内容
新增依赖
Store 模块重构
将原本 830 行的 src/infra/store.ts 拆分为模块化结构:
src/infra/
├── store.ts # 入口文件(向后兼容导出)
├── store/
│ ├── types.ts # 接口和类型定义
│ └── json-store.ts # JSONStore 文件存储实现
└── db/
├── sqlite/ # SQLite 实现
└── postgres/ # PostgreSQL 实现
CI 集成
Subagent Sandbox 泄漏修复 (src/core/agent.ts)
// Before: sandbox 未释放
const result = await subAgent.complete(config.prompt);
return result;
// After: 确保 sandbox 释放
try {
const result = await subAgent.complete(config.prompt);
return result;
} finally {
await (subAgent as any).sandbox?.dispose?.();
}
测试文件: agent-id.test.ts
问题: 期望 agt: 但实际是 agt-
修复方案: 修正为 agt- 并增加 Crockford Base32 验证
────────────────────────────────────────
测试文件: agent.test.ts
问题: CI 环境文件系统缓冲导致 resume 失败
修复方案: 增加 50ms 延迟等待写入完成
────────────────────────────────────────
测试文件: subagent.test.ts
问题: prompt 不够明确导致断言不稳定
修复方案: 优化 prompt 指令
────────────────────────────────────────
测试文件: resume-flow.test.ts
问题: 缺少 approval 处理
修复方案: 增加权限请求自动处理
────────────────────────────────────────
测试文件: room-collab.test.ts
问题: 清理时资源未释放
修复方案: 增加 sandbox dispose 和重试逻辑
────────────────────────────────────────
测试文件: multi-provider.test.ts
问题: 使用 Jest 语法
修复方案: 重构为 SDK 的 TestRunner
测试验证
破坏性变更
无。Store 模块保持完全向后兼容,现有导入路径无需修改。